Skip to content

std: make a lot of items crate private - #161612

Open
pacak wants to merge 1 commit into
rust-lang:mainfrom
pacak:unreachable-pub
Open

std: make a lot of items crate private#161612
pacak wants to merge 1 commit into
rust-lang:mainfrom
pacak:unreachable-pub

Conversation

@pacak

@pacak pacak commented Aug 23, 2026

Copy link
Copy Markdown
Contributor
  1. Enabled warn(unreachable_pub)
  2. Made a lot of things crate private, to the point when doing more required adding exceptions
  3. Removed warn(unreachable_pub)

So now this commit just tightens the current visibility without trying to enforce it.

View all comments

r? clarfonthey

@rustbot rustbot added the O-unix Operating system: Unix-like label Aug 23, 2026
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 23, 2026
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@pacak
pacak force-pushed the unreachable-pub branch 2 times, most recently from af7af07 to a6a436d Compare August 23, 2026 18:59
@rustbot rustbot added the O-windows Operating system: Windows label Aug 23, 2026
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@clarfonthey

Copy link
Copy Markdown
Contributor

So, looking at this, I'm going to nominate this for discussion alongside the clippy lints PR: #161328

Since they should go together, I think.

@rustbot label:I-libs-nominated

@rustbot rustbot added the I-libs-nominated Nominated for discussion during a libs team meeting. label Aug 23, 2026
@pacak

pacak commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

Sure.

Please no more obscure platform failures, please no more obscure platform failures, please no more obscure platform failures, please no more obscure platform failures...

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added the O-apple Operating system: Apple / Darwin (macOS, iOS, tvOS, visionOS, watchOS) label Aug 23, 2026
Comment thread library/std/src/sys/pal/windows/c/windows_sys.rs
@pacak
pacak force-pushed the unreachable-pub branch 2 times, most recently from 69a1881 to 21fd14f Compare August 23, 2026 21:41
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rustbot

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-bors

This comment has been minimized.

@rustbot

This comment has been minimized.

@clarfonthey

clarfonthey commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

So, I forgot to say the verdict from our meeting yesterday, which is everyone is cool with me making the call on this as a reviewer. We do have this turned on for core and alloc, so, I think it would be fine to keep the lint enabled here but maybe explicitly allow the lint for the os and sys modules as this makes it more of a mess.

Feel free to also allow the lint for any modules you find particularly annoying. We seem to have a standard of trying to keep it on by default and just allowing it for annoying bits.

@rustbot

rustbot commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@pacak

pacak commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

It's alive!!!

@clarfonthey clarfonthey removed the I-libs-nominated Nominated for discussion during a libs team meeting. label Aug 26, 2026
@clarfonthey

Copy link
Copy Markdown
Contributor

So, you still have a few changes inside the sys and os modules: were you hoping to keep those, or?

@pacak

pacak commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Right now I see them as "it was improved, but it is not enforced by clippy". I can probably go though a few iterations of whack-a-mole and enforce some more of them if you think its worth it. Probably tomorrow or something.

@clarfonthey

clarfonthey commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

I mostly mention it in the sense of, I figure if we're going to allow the lint for these modules, we probably shouldn't commit any of the changes for them, but I don't care too much either way. Mostly just curious if you had a compelling reason for it.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

Most of them don't need to be public, but there are scenarios where
thing is private on one platform but public on the other, so having a
lint on all the time gets complicated.

I enabled the lint, made a lot of things private to the point that
dealing with the rest required adding exceptions and disabled the lint
back again.
@pacak

pacak commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

So I ended up enabling the lint explicitly on Windows and Linux. This way stuff not used by obscure platforms that still needs to be public can stay public.

As for removing changes to files where the lint is not enforced - I think there are benefits to leave them as is - the goal is to have less stuff public after all.

@6-BennyLi-9

Copy link
Copy Markdown

Making some of the pub mod as pub(crate) mod is a good idea. But in my POV, we should lead the API alone. It may breaks Backward Compatibility rules,. Nobody knows if someone uses the API, then this pr may break the code.

@pacak

pacak commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Hmmm... Well, this PR started after I noticed some pub items inside of a non-pub module that wasn't reexported otherwise (unreachable pub). In my case this was producing error messages about potentially conflicting instances except that the type was strictly implementation detail and was not exported anywhere outside of the private module it was declared in, I checked.

Every pub(crate) here originates from a clippy complaining about pub(crate) being unreachable.

To break the code something needs to be used privately on both Linux and Windows, but needs to be visible outside of the crate via pub mod. Reexports won't work - it will fail to compile. For that you need to declare the same file mod twice with different visibility flags depending on a platform.

I did this

rg '^(pub |pub\(crate\) |)?mod \w+;' library/std

and don't see any.

Are there any specific scenarios you are worried about?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

O-apple Operating system: Apple / Darwin (macOS, iOS, tvOS, visionOS, watchOS) O-unix Operating system: Unix-like O-windows Operating system: Windows S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants